Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create dedicated cache and upstream metrics reports #189

Conversation

karlsassenberg
Copy link
Contributor

@karlsassenberg karlsassenberg commented Feb 2, 2023

Split Nginx Plus MetricsReport into four reports of the following types: CACHE_ZONE, UPSTREAMS, INSTANCE and SYSTEM. A single CACHE_ZONE report is created containing all StatsEntities with the dimension "cache_zone". A single UPSTREAMS report is created containing all StatsEntities with the dimension "upstream". The SYSTEM report contains all system metrics and the INSTANCE report contains all instance level metrics. Before only a single report was created of type SYSTEM which contained all metrics.

Proposed changes

Split out plus.cache.* and plus.http.upstream.* metrics out from the generated MetricReport, each into their own dedicated MetricReport.
Notes:
Default value of metrics.mode in default.go was changed to "aggregated" instead of "aggregation" to match default nginx-agent.conf and internal code usages.
Empty MetricReports are suppressed.
Added two new types MetricsReport MetricsReport_CACHE_ZONE and MetricsReport_UPSTREAMS.

Refactored Existing tests and wrote new ones.
Manual testing done with the attached nginx conf nginx_caches_upstreams.conf.gz
where the logs were used to confirm the MetricReports were generated as expected and the analytics api was queried to confirm that the metrics were being persisted to clickhouse. Tested both streaming and aggregated metrics mode.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • I have updated any relevant documentation (README.md)

@netlify
Copy link

netlify bot commented Feb 2, 2023

Deploy Preview for agent-public-docs canceled.

Name Link
🔨 Latest commit e15ea0a
🔍 Latest deploy log https://app.netlify.com/sites/agent-public-docs/deploys/641b494bda94a30008f0c0e1

@github-actions github-actions bot added chore Pull requests for routine tasks dependencies documentation Improvements or additions to documentation labels Feb 2, 2023
@karlsassenberg karlsassenberg force-pushed the add_seperate_cache_http_upstream_metrics_reports branch from 238fc98 to 099413b Compare February 23, 2023 17:08
test/integration/api/api_test.go Outdated Show resolved Hide resolved
test/component/agent_api_test.go Outdated Show resolved Hide resolved
src/core/metrics/metrics_util.go Outdated Show resolved Hide resolved
@karlsassenberg karlsassenberg force-pushed the add_seperate_cache_http_upstream_metrics_reports branch 2 times, most recently from c38aaf0 to e25d943 Compare March 7, 2023 17:34
Copy link
Collaborator

@dhurley dhurley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just have a few minor comments

src/core/metrics/metrics_util.go Outdated Show resolved Hide resolved
test/integration/api/api_test.go Outdated Show resolved Hide resolved
@karlsassenberg karlsassenberg force-pushed the add_seperate_cache_http_upstream_metrics_reports branch from e25d943 to 0735d86 Compare March 10, 2023 16:23
Copy link
Collaborator

@dhurley dhurley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
I verified that the metrics REST & gRPC APIs work as expected with this change.

@dhurley dhurley removed documentation Improvements or additions to documentation dependencies labels Mar 16, 2023
Split Nginx Plus MetricsReport into three groups of reports of the following types: CACHE_ZONE, UPSTREAMS and SYSTEM.
A CACHE_ZONE report is created for each distinct value of the dimension "cache_zone".
An UPSTREAMS report is created for each distinct value of the dimension "upstream".
A SYSTEM report contains all other metrics.
Split Nginx Plus MetricsReport into three groups of reports of the following types: CACHE_ZONE, UPSTREAMS and SYSTEM.
A CACHE_ZONE report is created for each distinct value of the dimension "cache_zone".
An UPSTREAMS report is created for each distinct value of the dimension "upstream".
A SYSTEM report contains all other metrics.
Split Nginx Plus MetricsReport into three groups of reports of the following types: CACHE_ZONE, UPSTREAMS and SYSTEM.
A CACHE_ZONE report is created for each distinct value of the dimension "cache_zone".
An UPSTREAMS report is created for each distinct value of the dimension "upstream".
A SYSTEM report contains all other metrics.
Split Nginx Plus MetricsReport into three groups of reports of the following types: CACHE_ZONE, UPSTREAMS and SYSTEM.
A CACHE_ZONE report is created for each distinct value of the dimension "cache_zone".
An UPSTREAMS report is created for each distinct value of the dimension "upstream".
A SYSTEM report contains all other metrics.
@karlsassenberg karlsassenberg force-pushed the add_seperate_cache_http_upstream_metrics_reports branch from 0735d86 to c45cdf2 Compare March 22, 2023 13:29
@github-actions github-actions bot added dependencies documentation Improvements or additions to documentation labels Mar 22, 2023
@oliveromahony oliveromahony merged commit 01d2002 into nginx:main Mar 27, 2023
@dhurley dhurley removed documentation Improvements or additions to documentation dependencies labels Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants